Tax receipt/verification page #2378
Conversation
…footer sections for donation receipt page
…s with translations
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…hen downloadUrl is available.
…ameters (dtn, year, challenge) - Implemented logic to fetch donation receipt data when valid query parameters are present. - Added type checks for dtn, year, and challenge before making the API request. - Updated the donation receipt data upon successful fetch. - Handled errors in case of failed API request.
- added next-intl keys for "donationAmount" and "name" with currency and type-based rendering. - defined proper types for Donor, Donation, and ReceiptData for stricter TypeScript typing. - moved fetchReceiptData logic from ReceiptDataSection to DonationReceiptLayout for better separation of concerns. - simplified UI by removing firstName and lastName fields, displaying only Name. - added skeleton structure for the receipt verification page to enhance loading experience.
…ents - added `formatReceiptData` helper to transform `ReceiptDataAPI` into `ReceiptData` with strict type validation. - Updated `ReceiptData` to include `operation` and `issuedDonations` fields. - Improved type safety for `DonorView`, `AddressView`, and donation-related properties. - Refactored components to align with the updated ReceiptData structure and context logic.
- Rename hasUserDataChanged to hasDonorDataChanged
…ataSection - Relocated the `confirmDonorData` logic to improve component structure and ensure relevant logic resides within `ReceiptDataSection`. -Introduce spinner for loading state.
There was a problem hiding this comment.
@sunilsabatp Please go through my feedback
- Increase the margin above the skeleton to make it in line with the top of the UI when the page renders (i.e. the top of the
h2element) - When url params are missed out, the user gets stuck on the loading screen, we could show an error message and redirect in a similar way to handling an error while fetching receipt data
There was a problem hiding this comment.
Should this be DonationReceiptContext instead of DonorReceiptContext?
The same applies for other places such names are used in this file.
| const data = await putRequest({ | ||
| tenant: tenantConfig.id, | ||
| url: `/app/donationReceipt/verify`, | ||
| data: { | ||
| dtn, | ||
| challenge, | ||
| year, | ||
| verificationDate: getVerificationDate(), | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Please add a type (T) for putRequest.
| if (!(dtn || year || challenge || router.isReady)) return; | ||
| if ( | ||
| typeof dtn !== 'string' || | ||
| typeof year !== 'string' || | ||
| typeof challenge !== 'string' | ||
| ) | ||
| return; |
There was a problem hiding this comment.
We should have explicit error handling in these cases. Currently I just see a loader which never disappears.
List down the possible scenarios for the url, and explicitly handle them. It isn't a good idea to make the user wait endlessly.
There was a problem hiding this comment.
Capitalize the file name for consistency i.e. DonationReceipt.module.scss.
public/static/locales/en/donate.json
Outdated
There was a problem hiding this comment.
Please don't use this translation file, create a new one just for the donation receipt flow.
We'd want to remove this eventually as it is part of legacy code when the donate platform was within the webapp.
| @mixin flex-layout( | ||
| $display: flex, | ||
| $flex-direction: null, | ||
| $justify-content: null, | ||
| $align-items: null, | ||
| $gap: null | ||
| ) { | ||
| display: $display; | ||
|
|
||
| @if $justify-content { | ||
| justify-content: $justify-content; | ||
| } | ||
|
|
||
| @if $align-items { | ||
| align-items: $align-items; | ||
| } | ||
|
|
||
| @if $gap { | ||
| gap: $gap; | ||
| } | ||
|
|
||
| @if $flex-direction { | ||
| flex-direction: $flex-direction; | ||
| } | ||
| } |
There was a problem hiding this comment.
I understand the intention here, but this makes the code less readable. I need to remember the parameter order while reading the code, or keep referring back here.
I suggest removing this and using the required css directly with standard flexbox properties- even if it is longer, it is actually easier to read.
| const t = useTranslations('Donate'); | ||
| const tCountry = useTranslations('Country'); | ||
| const { country, zipCode, city, address1, address2 } = address; | ||
| const { type, name, tin } = donor; |
There was a problem hiding this comment.
Suggest not destructuring donor for readability.
| </span> | ||
| <span>{name}</span> | ||
| </div> | ||
| {tin && ( |
There was a problem hiding this comment.
Avoid using this pattern. It is easy to have leaked renders.
If using && to conditionally render, make sure the left side is a boolean value.
| <div className={styles.details}> | ||
| <div className={styles.donorName}> | ||
| <span className={styles.header}> | ||
| {t('donationReceipt.name', { | ||
| type, | ||
| })} | ||
| </span> | ||
| <span>{name}</span> | ||
| </div> | ||
| {tin && ( | ||
| <div className={styles.tin}> | ||
| <span className={styles.header}> | ||
| {t('donationReceipt.taxIdentificationNumber')} | ||
| </span> | ||
| <span>{tin}</span> | ||
| </div> | ||
| )} | ||
| </div> | ||
| <div className={styles.address}> | ||
| <span className={styles.header}>Address</span> | ||
| <address> | ||
| {address1},{cityStatePostalString} | ||
| </address> | ||
| {address2 && ( | ||
| <address> | ||
| {address2},{cityStatePostalString} | ||
| </address> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Let's call the outer divs - donorInfo and addressInfo, just so that the structure is easy to understand.
| } | ||
|
|
||
| const DonorDetails = ({ donor, address }: Props) => { | ||
| if (address === undefined || donor === undefined) return null; |
There was a problem hiding this comment.
I suggest removing this condition, updating the types for donor and address to not allow undefined. We would not want to show just donation details/receipt actions and no donor details.
If donor or address are absent in the data, it should be treated as an error scenario, an appropriate error message should be shown (invalid data encountered), along with a way to get in touch to fix things if needed.
Perhaps this logic could be kept within ReceiptDataSection/DonorReceiptLayout instead, and we could create a component to show an error message instead of the rest of ReceiptDataSection.
…ule.scss to match file name convention.
- rename ReceiptListRedirect to ReceiptFooterSection
Title: Implement Tax Receipt Verification and Download Feature
Description:
Added a new verification interface for tax receipt downloads that:
To DO:
How to test:
Note : Use the
development API'sto test the featureFigma design ->
https://www.figma.com/design/lT7AVzzAUM0NyThCyvacob/MyForest-Dashboard?node-id=4824-1294&t=kXNroXn6Xt7SbaaB-0Route ->
/verify-receipt-data?dtn={dtn}&challenge={challenge}&year={year}